Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TaskRunner support for shutting down queues #5502

Merged
merged 1 commit into from
Sep 27, 2019
Merged

Conversation

swankjesse
Copy link
Collaborator

Shutting down queues makes it easier to implement HTTP/2
shutdown because we can enqueue everywhere and centralize
the logic that decides whether we're shutdown or not.

Use this new functionality to implement HTTP/2 on task queues.
It's mostly a drop-in replacement, though opting-into cancel
looks like a mistake when it's what we do most of the time.

When testing this the OkHttpClientTestRule was failing because
tasks were incomplete. I was puzzled by this until I realized
that the OkHttpClientTestRule performs that validation before
we stop the MockWebServer. I changed MockWebServer to have its
own TaskRunner and that made the problem go away.

Shutting down queues makes it easier to implement HTTP/2
shutdown because we can enqueue everywhere and centralize
the logic that decides whether we're shutdown or not.

Use this new functionality to implement HTTP/2 on task queues.
It's mostly a drop-in replacement, though opting-into cancel
looks like a mistake when it's what we do most of the time.

When testing this the OkHttpClientTestRule was failing because
tasks were incomplete. I was puzzled by this until I realized
that the OkHttpClientTestRule performs that validation before
we stop the MockWebServer. I changed MockWebServer to have its
own TaskRunner and that made the problem go away.
@swankjesse swankjesse merged commit 253d2ea into master Sep 27, 2019
@swankjesse swankjesse deleted the jwilson.0926.http2 branch September 27, 2019 21:26
@swankjesse swankjesse mentioned this pull request Sep 29, 2019
15 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants